feat(net): credential injection primitives — inject an auth secret in the proxy (RFC #66) by dzerik · Pull Request #127 · multikernel/sandlock

dzerik · 2026-07-06T11:48:53Z

Implements RFC #66, Phase 1, Layer 2 (transparent mode). This is the bottom-up primitive slice — the generic mechanism, not the --service openai presets (those are a later slice).

What it does

A named secret lives only in the supervisor. When the sandboxed child makes a request to a matching upstream, the secret is rendered into an auth header (or query parameter) inside the MITM proxy — after http_acl_check, so a denied request never touches the secret — and the child never sees the real value.

sandlock run \
  --http-allow "* api.openai.com/*" --http-inject-ca \
  --credential openai=env:OPENAI_API_KEY \
  --http-inject "* api.openai.com/* bearer openai" \
  -- agent ...

Design

Secret containment. SecretString is zeroed on drop (volatile writes), has a redacted Debug, and deliberately implements no Display/ToString/serde::Serialize and is unreachable through any Error chain — so a stray format!("{}", ..) or a serialized policy can't re-open the leak the type exists to close. It flows to the proxy as Arc<Vec<InjectRule>> and is #[serde(skip)] on the sandbox (re-loaded from source per build, never persisted).
ACL before inject. AclService::handle applies the first matching rule only after the ACL passes; the deny path returns before any rendering. Audit records the credential name, never the value.
OnExistingHeader defaults to Replace. The proxy owns the credential, and every major SDK requires an API key to be set and always sends its own placeholder Authorization. Keeping it would forward the placeholder and never inject — so the rule's target header is replaced by default; pass a trailing add-only to keep a child-set value instead.
Sources. env: / file: / fd: (a trailing newline is stripped); literal: is rejected (leaks via ps / shell history). file:/fd: are capped at 64 KiB; fd: refuses the std streams and reads through a dup so the caller's fd stays open. Env-sourced secrets are stripped from the child's environment post-fork, so the agent can't read the value straight out of its own env.

Safety rails

--http-inject without an ACL proxy is rejected; without a CA it warns that only plaintext HTTP is intercepted (a rule for an HTTPS host would silently no-op otherwise).
A secret that can't render into a header (e.g. contains CRLF) fails the request rather than forwarding it uncredentialed.
A checkpoint is rejected while injection is configured, since the non-serialized secrets can't be restored into the image.

Auth shapes

bearer · basic:<user> (base64) · header:<name> / apikey:<name> · query:<param> (raw bytes percent-encoded, so a binary key isn't corrupted).

Tests

Unit: SecretString redaction, source loading (incl. literal:/std-fd rejection and the 64 KiB cap), every AuthShape, the Replace default vs add-only, query AddOnly de-dup, a CRLF secret failing the request, and resolve_inject_rules Arc de-dup + env-strip collection.
Integration: a credential is injected into the upstream while the child carries none; a denied request never renders the secret (403, upstream never contacted); an env: credential is scrubbed from the child.
HTTPS/MITM injection runs the same scheme-agnostic apply path; a TLS end-to-end test needs a CA-trust harness and is tracked as a follow-up (noted in the test file).

Deferred to later slices

--service openai (Layer 1) presets, phantom-token env swap, explicit proxy mode, body-level injection, and a structured audit log (the current audit is a by-name stderr line).

… the proxy (RFC multikernel#66) Phase 1, Layer 2 (transparent mode): a named secret lives only in the supervisor and is rendered into a matching request's auth header (or query parameter) inside the MITM proxy — after the ACL check, so a denied request never touches the secret — while the sandboxed child never sees the real value. New `credential` module: - `SecretString`: zeroed on drop (volatile writes), redacted `Debug`, and deliberately no `Display`/`ToString`/`Serialize` and unreachable through any `Error` chain, so a stray `format!("{}", ..)` can't re-open the leak. - Sources `env:`/`file:`/`fd:` (a trailing newline is stripped); `literal:` is rejected — it leaks via ps / shell history. `file:`/`fd:` are capped at 64 KiB and `fd:` refuses the std streams (0/1/2) and reads through a dup. - `AuthShape` renderings: `bearer`, `basic:<user>` (base64), `header:<name>` / `apikey:<name>`, `query:<param>`. Injected headers are marked sensitive; a secret that can't render (e.g. CRLF) fails the request rather than forwarding it uncredentialed. - `OnExistingHeader` defaults to `Replace`: the proxy owns the credential, and SDKs (openai, anthropic, …) always send a placeholder `Authorization`, so keeping it would forward the placeholder and never inject. Pass the trailing `add-only` token to keep a value the child set instead. Wiring: `--credential NAME=SOURCE` and `--http-inject "METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]"` flow through the builder to a resolved `Arc<Vec<InjectRule>>` on the sandbox (never serialized — `#[serde(skip)]`; secrets are re-loaded from source per build, not persisted). `AclService` applies the first matching rule to the outbound request after `http_acl_check` and records the credential *name* — never its value. `--http-inject` requires an HTTP ACL proxy (rejected otherwise); with no CA it warns that only plaintext HTTP is intercepted. A checkpoint is rejected while injection is configured (the non-serialized secrets can't be restored into the image). Env-sourced secrets are stripped from the child's environment post-fork, so the agent can't read the real value straight out of its own env instead of relying on proxy-side injection. Deferred to later slices: `--service openai` (Layer 1), phantom-token env swap, explicit proxy mode, body-level injection, and a structured audit log (the current audit is a by-name stderr line). Tests: unit coverage of `SecretString` redaction, source loading (incl. `literal:`/std-fd rejection and the 64 KiB cap), every `AuthShape`, the `Replace` default vs `add-only`, query encoding, a CRLF secret failing the request, and rule parsing incl. Arc dedup + env-strip collection; integration tests that a credential is injected into the upstream while the child carries none, that a denied request never renders the secret, and that an `env:` credential is scrubbed from the child. HTTPS/MITM injection is exercised by the same scheme-agnostic path but a TLS end-to-end test is a follow-up.

congwang-mk

Design looks solid overall: ACL-before-inject is structural, SecretString containment is well closed, and reusing HttpRule keeps one matching semantics. Four inline findings below. Two smaller notes not worth inline threads: (1) with a CA configured, a rule matching a port-80 host still injects into cleartext with no warning (the build-time warning only fires when no CA is set); (2) refusing fd:0 blocks the common stdin secret pattern (docker/systemd pipes), consider allowing it or documenting the <(...) workaround.

congwang-mk · 2026-07-06T18:57:37Z

+    // Remove env vars whose value was loaded into the supervisor as a credential
+    // source, so the agent can't read the real secret straight from its own
+    // environment (this is the child; the mutation is process-local and post-fork).
+    for name in &sandbox.inject_env_strip {


This strip runs after the sandbox.env set loop, so it also deletes an explicitly passed placeholder (--env OPENAI_API_KEY=dummy). SDKs refuse to start without the var set, which is exactly the placeholder flow the Replace default is designed for, so the canonical env: workflow breaks before any request is made. Strip the inherited env first, then apply sandbox.env: the inherited real secret is still removed and deliberate user config is honored.

congwang-mk · 2026-07-06T18:57:37Z

+        let uri = &parts.uri;
+        let path = uri.path();
+        let existing = uri.query();
+        // Honor AddOnly: don't append a param the request already carries.


Replace does not replace for the query shape: the existing param is only checked under AddOnly, so the default path appends a duplicate (key=child&key=sk-real, as the test pins). Most frameworks read the first occurrence, so the upstream authenticates against the child's placeholder and returns 401; it is also parameter-pollution ambiguity. On Replace, filter out existing param= pairs before appending, mirroring headers.insert.

congwang-mk · 2026-07-06T18:57:37Z

+        // `apikey:<header>` and `header:<name>` are the same rendering.
+        ("header" | "apikey", Some(name)) if !name.is_empty() => AuthShape::Header { name: name.to_string() },
+        ("query", Some(param)) if !param.is_empty() => AuthShape::Query { param: param.to_string() },
+        _ => {


A username containing : is accepted here (basic:a:b yields username a:b), which shifts the credential boundary in the encoded user:pass: the upstream sees user a, password b:<secret>. RFC 7617 forbids : in the user-id; reject it at parse time.

congwang-mk · 2026-07-06T18:57:37Z

+pub fn parse_auth(spec: &str, credential: &str) -> Result<AuthShape, SandboxError> {
+    let (kind, arg) = match spec.split_once(':') {
+        Some((k, a)) => (k, Some(a)),
+        None => (spec, None),


Stale doc: says it returns (shape, credential_name) but the function returns only the AuthShape.

- context.rs: strip inherited credential env vars *before* applying `sandbox.env`, so a deliberate placeholder (`--env OPENAI_API_KEY=dummy`, which SDKs need set to start) survives while the inherited real secret is still removed. Previously the strip ran after and deleted the placeholder, breaking the canonical `env:` flow before any request. - credential.rs: `Replace` now actually replaces for the query shape — existing occurrences of the target param are dropped before appending, instead of producing a duplicate (`key=child&key=sk-real`) where a framework reading the first occurrence would authenticate against the child's value. The test is corrected to assert replacement, not the duplicate it pinned. - credential.rs: reject ':' in a `basic:<user>` user-id (RFC 7617) — it would shift the `user:pass` boundary and leak part of the secret into the password. - credential.rs: drop the stale `parse_auth` doc line claiming a tuple return.

congwang-mk · 2026-07-10T04:41:43Z

        let (mut parts, body) = req.into_parts();
        parts.uri = uri;
+
+        // ACL passed: attach a credential if a rule matches. First match wins.


Do we need to consider multi-header injection? Or is it out of scope?

congwang-mk · 2026-07-10T04:49:59Z

Another potential issue: without CA, we get HTTP port 80, this might not be expected by this injection?

congwang-mk · 2026-07-10T04:57:38Z

A minor name issue: --http-inject is a bit similar to the existing --http-inject-ca, maybe rename it to --http-auth (or any other better name)?

dzerik · 2026-07-10T13:36:14Z

Thanks for both notes. Status plus one design question for you.

--http-inject → --http-auth: agreed — will rename to disambiguate from --http-inject-ca.

Cleartext (port 80) injection — this is the real call, and I'd rather not over-restrict it, because plaintext HTTP at the app layer is legitimately secure in a common deployment: a service-mesh sidecar (Istio/Linkerd mTLS) terminates TLS, so the app speaks plain http to 127.0.0.1/the sidecar while the wire is mTLS. Refusing cleartext injection outright would break that.

service.rs already knows the scheme per request ("https" for the MITM path, "http" for plaintext), so we can act on it at request time rather than only warning at build time. Options, your call on surface:

(a) warn-at-request-time, still inject — no new flag; warn once per host when injecting over http. Closes the "silent in the with-CA case" gap and keeps the mesh use case working.
(b) refuse cleartext by default + opt-in --http-auth-allow-cleartext — fail-closed, but adds a flag.
(c) refuse cleartext unconditionally — no flag, but breaks the mesh-mTLS case above.

I lean (a) (minimal surface, keeps legit cleartext working, makes the leak visible instead of silent), but it's your surface — happy to wire (b) if you'd rather fail-closed.

Multi-header (service.rs:158): intentionally first-match-wins, one credential per request. Multiple simultaneous auth headers seemed unusual, so I kept it out of scope — glad to add it if you have a case in mind.

congwang-mk · 2026-07-10T18:27:46Z

Emitting a warning on CLI side is fine, how about API side? :)

congwang-mk · 2026-07-10T18:28:52Z

BTW, while you are on it, please update README.md and docs/sandbox-reference.md together in this PR.

dzerik · 2026-07-10T21:06:19Z

Pushed the three follow-ups:

Renamed --http-inject → --http-auth (and the SandboxBuilder::http_auth field/method to match), so it no longer reads as a sibling of --http-inject-ca. The CA flags (--http-inject-ca / --http-ca) are untouched.
The cleartext warning is now API-side. You're right that a CLI-only warning misses library callers — so it lives in the transparent proxy (transparent_proxy/service.rs), which is core and runs for every caller. It fires at request time (the only place the scheme is known), latched once per run, when a credential is actually injected over plaintext http. It names only the credential and host — never the secret value — and is a warning only, the request still proceeds.

I kept the existing build-time warning as well: it covers a different risk (a CA isn't set, so credentials scoped to an HTTPS host would be silently dropped), whereas the new one fires when a credential actually goes out in cleartext.
Docs — a README example plus a Credential injection section in docs/sandbox-reference.md, in this PR.

…eartext, docs Address review follow-ups: - Rename the CLI flag --http-inject to --http-auth (and the SandboxBuilder::http_auth field/method to match) so it no longer reads as a sibling of --http-inject-ca, which is an unrelated CA-provisioning flag. The CA flags (--http-inject-ca / --http-ca) are untouched. - Warn when a credential is injected over cleartext http. The warning is emitted in the transparent proxy (core), at request time where the scheme is known, so it fires for library/API callers and not just the CLI. It is latched once per run, names only the credential and host (never the secret value), and is a warning only -- the request still proceeds. The existing build-time "no CA set" warning covers a different risk and is kept. - Document --http-auth and credential injection in README.md and docs/sandbox-reference.md.

congwang-mk · 2026-07-10T22:57:11Z

+        if !loaded.contains_key(p.name) {
+            loaded.insert(p.name, Arc::new(load_secret(p.source)?));
+            if let Some(var) = p.source.strip_prefix("env:") {
+                if !env_strip.iter().any(|v| v == var) {


If a credential is defined but not referenced, this would leak it to processes in the sandbox?

…edentials Review follow-ups: - SECURITY: reject a split-host request in the proxy. handle() keyed the ACL check, host verification, and the credential match on the URI-authority host but rebuilt the outbound request from the Host header, and the upstream client routes by that. A child could send `GET https://allowed/...` with `Host: attacker` to have the secret injected for the allowed host yet forwarded (carrying the credential) to an attacker-chosen one -- cross-origin credential exfiltration plus an egress-ACL bypass. Fail closed when the URI host and the Host header disagree (split_host_rejected), before verify/ACL/inject. - Strip the env var of EVERY declared `env:` credential from the child, not only the ones an --http-auth rule references. A declared-but-unreferenced `--credential X=env:SECRET` was left in the child's environment (readable directly) and could also reach the on-disk checkpoint image via the child's memory. Populating env_strip from all declarations closes both. - Warn when a `file:` credential path sits inside an fs-read grant (the child can read it directly) -- env: is stripped and fd: is dup-read, but file: had no backstop. - apply() now returns Injected/Skipped so an add-only no-op is logged truthfully ("kept caller-supplied credential") instead of a false "injected" audit line. Unit tests: split-host guard, unreferenced-env strip.

dzerik · 2026-07-11T14:38:18Z

Pushed 00b83b8. While addressing your env-strip question I ran a deeper self-audit of the whole feature and found a more serious issue I want to flag directly.

Critical — cross-origin credential exfiltration (now fixed). The proxy keyed the ACL check, verify_host, and the credential match on the URI-authority host (host), but rebuilt the outbound request from the Host header (host_hdr) — and the upstream client routes by that. So a hostile child could send GET https://api.openai.com/v1 with Host: attacker.example: the secret is injected for the allowed host, then the request (carrying Authorization: Bearer <secret>) is forwarded to attacker.example. That's cross-origin exfiltration of the exact credential the feature promises the child never sees, plus an egress-ACL bypass, in both plaintext and MITM modes. Worth flagging that the root — host vs host_hdr divergence and forwarding by the Host header — predates this PR in the transparent proxy; credential injection just turns it into a secret leak, so it may deserve a look independent of this feature. Fixed here by failing closed when the URI-authority host and the Host header disagree (split_host_rejected), before verify/ACL/inject; origin-form requests are unaffected. Unit test added.

Your env-strip question — confirmed, fixed. You're right: env_strip was built only from referenced credentials, so a declared-but-unreferenced --credential X=env:SECRET stayed in the child's environment. It's now stripped for every declared env: credential regardless of reference. Two things fall out of the same root:

it also left the secret in the child's memory → the on-disk checkpoint image (capture.rs guards on inject, which is empty in the unreferenced case) — the strip-all fix closes that too;
the supervisor still holds the secret in its own /proc/<pid>/environ, reachable by a child granted /proc with no PID-ns. That one is a deeper property of the supervisor having to hold the secret at all, so I'm flagging it rather than fixing it here.

Two smaller ones while in there: file: credentials had no protection analogue (env: stripped, fd: dup-closed) — now a build-time warning if the secret file sits inside an fs-read grant. And apply() returned Ok(()) for both a real injection and an add-only no-op, so the audit line claimed "injected" when it actually kept the child's header — it now returns Injected/Skipped and logs truthfully.

Left multi-header out of scope as before (first-match-wins). Happy to adjust any of these.

…t follow-up) A self-audit of the previous fix found #1 incomplete and surfaced more gaps: - The split-host guard compared uri.host() against a split(':') view of the Host header while the request was forwarded using the full re-parsed Host authority, so `Host: allowed:x@attacker` (userinfo) or an origin-form request still routed the injected secret to an attacker host. Replaced with request_authority(): parse the Host header and any URI authority as http::uri::Authority, reject userinfo and a URI-vs-Host mismatch, and drive verify/ACL/inject AND the outbound request from that ONE authority. Also fixes IPv6-literal handling. - The file: overlap warning now covers every grant that exposes the secret -- fs_writable (write access includes read), fs_mount host paths, and the chroot root -- not just fs_readable. - Query Replace/AddOnly now match the param NAME, so a value-less `?key` is treated as the target instead of appending `key&key=secret` (which a first-occurrence-reading upstream would authenticate with the empty value). - Allow fd:0 (stdin), the most portable secret-passing pattern, rejecting only fd:1/fd:2. Reject --http-auth specs with more than 5 tokens. Unit tests: request_authority (userinfo/split/origin-form rejection), value-less query param.

dzerik · 2026-07-11T15:22:56Z

Correction on my previous comment — the split-host fix I described was incomplete; a self-audit found two live bypasses. Pushed a proper fix (c4c3fd6).

The earlier guard compared uri.host() with a split(':') view of the Host header, but the outbound request re-parses the full Host header as an authority (whose .host() strips userinfo), so security-host and forward-host could still diverge — origin-form, and Host: allowed:x@attacker.example (userinfo), both still exfiltrated. Replaced with a single request_authority() that parses both the Host header and any URI authority as http::uri::Authority, rejects userinfo and a URI-vs-Host mismatch, and drives verify/ACL/inject and the outbound request from that one value — so the host a credential is matched for is exactly the host it's sent to, in every request form. Fixes IPv6 literals for free.

The same audit closed three more while I was in there: the file: overlap warning now covers fs_writable (write access includes read), fs_mount host paths, and the chroot root, not just fs_readable; query Replace/AddOnly now match the param name, so a value-less ?key is the target rather than appending key&key=secret; and fd:0 (stdin) is allowed per your earlier note (only fd:1/fd:2 refused), with --http-auth rejecting >5 tokens.

Also confirmed the supervisor-side /proc/<pid>/environ residual I mentioned isn't reachable: handle_proc_open's per-PID filter already returns EACCES for any pid not in the sandbox ProcessIndex, and the supervisor is never registered.

dzerik force-pushed the feat/credential-injection branch from c8db97b to a0eeac8 Compare July 6, 2026 12:00

congwang-mk reviewed Jul 6, 2026

View reviewed changes

congwang-mk reviewed Jul 10, 2026

View reviewed changes

dzerik force-pushed the feat/credential-injection branch from 5f9d5d7 to 5667e57 Compare July 10, 2026 21:17

dzerik force-pushed the feat/credential-injection branch from 5667e57 to 3bd2fe9 Compare July 10, 2026 21:24

congwang-mk reviewed Jul 10, 2026

View reviewed changes

congwang-mk merged commit 143742e into multikernel:main Jul 11, 2026
13 checks passed

congwang-mk mentioned this pull request Jul 11, 2026

credential: address the four remaining findings from the PR #127 review #138

Open

Uh oh!

Conversation

dzerik commented Jul 6, 2026

What it does

Design

Safety rails

Auth shapes

Tests

Deferred to later slices

Uh oh!

congwang-mk left a comment

Choose a reason for hiding this comment

Uh oh!

congwang-mk Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

congwang-mk Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

congwang-mk Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

congwang-mk Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

congwang-mk Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

congwang-mk commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

congwang-mk commented Jul 10, 2026

Uh oh!

dzerik commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

congwang-mk commented Jul 10, 2026

Uh oh!

congwang-mk commented Jul 10, 2026

Uh oh!

dzerik commented Jul 10, 2026

Uh oh!

congwang-mk Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

dzerik commented Jul 11, 2026

Uh oh!

dzerik commented Jul 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

congwang-mk commented Jul 10, 2026 •

edited

Loading

dzerik commented Jul 10, 2026 •

edited

Loading